Conversation
- jpa의존성 추가 - postgresql로 db 및 driver변경에 따른 의존성 수정
도메인, dao(repository), 서비스, 컨트롤러 작성 - dao(repository) : JPA사용하여 구현 - 서비스 : 인터페이스 설계 후, 구현체 작성 - 컨트롤러 : 공통 응답 포맷 만들어서 resposneEntity에 담아서 응답
요청과 응답 dto작성 inner class 활용함
공통응답 포맷 작성
- 예외처리 핸들러 1개 - 에러 커스텀 가능하도록 ErrorCode 인터페이스와 BusinessException 인터페이스 작성 - 예외와 에러코드는 작성한 2개의 인터페이스 상속받아서 작성
build 파일에서 mysql드라이버 부분 제거
🚀 검토 요약개요이 PR은 폴더 관리 모듈의 포괄적인 리팩토링과 새로운 에러 처리 인프라 구축을 수행합니다. 표준화된 API 응답 래퍼(ApiResponse), 커스텀 예외 계층(BusinessException, FolderException), 에러 코드 관리(ErrorCode 인터페이스)를 도입합니다. 기존 MyBatis 기반 폴더 모듈(FolderService)을 완전히 제거하고 JPA 기반의 새로운 아키텍처(MemberFolderService)로 교체합니다. 컨트롤러, 서비스, DAO, 엔티티, DTO가 /api/folders 엔드포인트 하의 새로운 패턴을 사용하도록 업데이트됩니다. 순서 다이어그램sequenceDiagram
participant Client
participant MemberFolderController
participant MemberFolderService
participant MemberFolderJpaDao
participant Database
participant GlobalExceptionHandler
Client->>MemberFolderController: POST /api/folders (Create)
MemberFolderController->>MemberFolderService: create(ownerMemberId, parentFolderId, folderName, description)
alt 검증 성공
MemberFolderService->>MemberFolderJpaDao: save(MemberFolder)
MemberFolderJpaDao->>Database: INSERT member_folder
Database-->>MemberFolderJpaDao: memberFolderId
MemberFolderJpaDao-->>MemberFolderService: saved entity
MemberFolderService-->>MemberFolderController: Long (folderId)
MemberFolderController-->>Client: 201 Created + ApiResponse<Long>
else 폴더명 검증 실패
MemberFolderService->>GlobalExceptionHandler: throw BusinessException
GlobalExceptionHandler->>Client: 400/500 + ApiResponse<ErrorResponse>
end
sequenceDiagram
participant Client
participant MemberFolderController
participant MemberFolderService
participant MemberFolderJpaDao
participant Database
participant GlobalExceptionHandler
Client->>MemberFolderController: GET /api/folders/{folderId}
MemberFolderController->>MemberFolderService: get(memberFolderId)
alt 폴더 존재
MemberFolderService->>MemberFolderJpaDao: findById(memberFolderId)
MemberFolderJpaDao->>Database: SELECT * FROM member_folder
Database-->>MemberFolderJpaDao: MemberFolder entity
MemberFolderJpaDao-->>MemberFolderService: Optional<MemberFolder>
MemberFolderService-->>MemberFolderController: MemberFolder
MemberFolderController-->>Client: 200 OK + ApiResponse<MemberFolderResponses>
else 폴더 미존재
MemberFolderService->>GlobalExceptionHandler: throw FolderException.NotFound
GlobalExceptionHandler->>Client: 404 + ApiResponse<ErrorResponse>
end
📋 코드 리뷰 예상 소요 시간🎯 4 (복잡) | ⏱️ ~45분 🐰 축하 시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 18
🤖 Fix all issues with AI agents
In `@build.gradle`:
- Line 37: Remove the duplicate dependency declaration of
'org.springframework.boot:spring-boot-starter-data-jpa' (keep only one
occurrence) and fix the inconsistent indentation on the remaining declaration by
converting the leading spaces to a tab so it matches the file's existing
indentation style; locate the duplicate by searching for the exact dependency
string and update the earlier occurrence's indentation accordingly.
In `@src/main/java/com/web/SearchWeb/config/CommonErrorCode.java`:
- Around line 9-11: The enum CommonErrorCode currently defines
INTERNAL_SERVER_ERROR with code "C006"; verify the intended numbering scheme and
either adjust the code value on INTERNAL_SERVER_ERROR to the correct sequence
(e.g., "C001" if it should be first) or add/document the missing placeholders
for C001–C005 and a comment explaining the numbering convention; update the enum
constant (INTERNAL_SERVER_ERROR) and associated documentation/comments to
reflect the chosen, consistent code scheme.
In `@src/main/java/com/web/SearchWeb/config/GlobalExceptionHandler.java`:
- Around line 26-33: Add a specific handler for IllegalArgumentException in
GlobalExceptionHandler so client validation errors from
MemberFolderServiceImpl.validateFolderName() return 400 instead of being caught
by the generic handleException(Exception). Implement a new method (e.g.,
handleIllegalArgumentException) annotated with
`@ExceptionHandler`(IllegalArgumentException.class) that logs at WARN/INFO and
returns
ResponseEntity.status(HttpStatus.BAD_REQUEST).body(ApiResponse.fail(...)) using
an appropriate client-error CommonErrorCode (or add one if missing);
alternatively, change MemberFolderServiceImpl.validateFolderName() to throw a
BusinessException subclass so the existing BusinessException handler handles it.
In
`@src/main/java/com/web/SearchWeb/folder/controller/dto/MemberFolderRequests.java`:
- Around line 8-13: Remove the ownerMemberId field from the
MemberFolderRequests.Create DTO (so it is no longer accepted from the request
body) and update the folder creation flow to derive the owner ID from the
authenticated principal instead (e.g., obtain memberId from
SecurityContextHolder or an `@AuthenticationPrincipal` argument in the controller
method that handles Create). Ensure the controller no longer reads
Create.ownerMemberId but passes the principal-derived memberId into the
service/repository create method (keeping parentFolderId, folderName,
description on Create unchanged) so owner assignment is done server-side and
IDOR is eliminated.
- Around line 3-28: The DTOs in MemberFolderRequests (inner classes Create,
Update, Move) lack Bean Validation and use public mutable fields; change each
DTO to encapsulate fields (make them private and provide getters/setters or
apply Lombok annotations like `@Getter/`@Setter/@NoArgsConstructor) and add
appropriate javax.validation annotations: for Create.folderName use `@NotBlank`
and `@Size`(max=...) (and `@NotNull` for ownerMemberId), for Update.folderName use
`@NotBlank/`@Size as needed, and for Move.newParentFolderId use `@NotNull` if moving
is required (or omit if null allowed); finally ensure controller methods that
accept these DTOs annotate the `@RequestBody` parameters with `@Valid` so validation
runs on entry.
In
`@src/main/java/com/web/SearchWeb/folder/controller/dto/MemberFolderResponses.java`:
- Line 9: The DTO class MemberFolderResponses is named in plural but represents
a single folder response; rename the class to MemberFolderResponse to match
semantics and update all references/usages (imports, constructors, builders,
JSON serializers, controller/service method return types, and any tests) to use
MemberFolderResponse instead of MemberFolderResponses; if any code expects a
collection, convert those signatures to use List<MemberFolderResponse> or
Collection<MemberFolderResponse> accordingly and run/refactor compilation fixes
and imports.
In
`@src/main/java/com/web/SearchWeb/folder/controller/MemberFolderController.java`:
- Around line 89-94: Change the DELETE handler to return HTTP 204 No Content
instead of a 200 with a body: update the MemberFolderController.delete method
signature to return ResponseEntity<Void> (remove the ApiResponse wrapper), call
memberFolderService.delete(folderId) as before, and return
ResponseEntity.noContent().build(); ensure imports and any callers are updated
to expect no body. This references MemberFolderController.delete and
memberFolderService.delete.
- Around line 31-41: Add Bean Validation to the create endpoint by annotating
the controller parameter in MemberFolderController.create with `@Valid` and
applying appropriate validation annotations to fields in
MemberFolderRequests.Create (e.g., `@NotNull` on ownerMemberId, `@NotBlank` on
folderName, leave parentFolderId nullable). Also add or update a handler in
GlobalExceptionHandler to catch MethodArgumentNotValidException and return a
clear ApiResponse error so validation failures produce controlled HTTP 400
responses.
In `@src/main/java/com/web/SearchWeb/folder/dao/MemberFolderJpaDao.java`:
- Around line 7-17: The delete path currently calls
MemberFolderServiceImpl.delete() which only invokes deleteById() and thus leaves
child folders orphaned or causes FK constraint failures; add a repository method
like MemberFolderJpaDao.deleteAllByParentFolderId(Long parentFolderId) and
update MemberFolderServiceImpl.delete() to recursively delete child folders (or
iterate breadth-first using deleteAllByParentFolderId), or alternatively
annotate the MemberFolder entity with cascade delete/orphanRemoval (e.g.,
orphanRemoval=true or `@Cascade`(CascadeType.ALL)) to ensure children are removed
automatically—pick one approach and implement the repository method or entity
cascade and adjust the service to use it.
In `@src/main/java/com/web/SearchWeb/folder/domain/MemberFolder.java`:
- Around line 23-34: Add audit fields to the MemberFolder entity: declare
createdAt and updatedAt (e.g., Instant or LocalDateTime) and annotate them with
`@CreatedDate` and `@LastModifiedDate` respectively; annotate the entity with
`@EntityListeners`(AuditingEntityListener.class) so JPA will populate them, and
ensure Spring Data JPA auditing is enabled in the application (EnableJpaAuditing
in a config class) so MemberFolder's createdAt/updatedAt are automatically
maintained on persist/update.
- Around line 15-18: The class MemberFolder currently uses `@AllArgsConstructor`
with default (public) visibility together with `@Builder`, exposing a public
all-args constructor; change the AllArgsConstructor to be private by replacing
`@AllArgsConstructor` with `@AllArgsConstructor`(access = AccessLevel.PRIVATE) so
the builder remains usable but the public all-args constructor is not exposed
(target the MemberFolder class annotations).
- Around line 44-46: In MemberFolder.changeParent, prevent setting a folder as
its own parent by validating that newParentFolderId is not equal to
this.memberFolderId; if they are equal, throw an IllegalArgumentException (or a
domain-specific exception) instead of assigning to parentFolderId. Update the
method (changeParent) to perform this check before assigning this.parentFolderId
= newParentFolderId; keep additional indirect-cycle validation for the service
layer that inspects ancestor chains.
In `@src/main/java/com/web/SearchWeb/folder/error/FolderErrorCode.java`:
- Around line 10-16: FolderErrorCode defines DUPLICATE_FOLDER_NAME but
MemberFolderServiceImpl lacks duplicate-name validation; add a check in
MemberFolderServiceImpl (in create/update folder methods) to query the
repository for existing folder names for the member (e.g., using
FolderRepository methods or findByMemberAndName) and, if a conflict exists,
throw your domain exception using FolderErrorCode.DUPLICATE_FOLDER_NAME; also
implement the PR-mentioned cap (e.g., MAX_FOLDERS = 64) by counting existing
member folders before create and throw a suitable error if the limit is reached.
In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderService.java`:
- Around line 13-15: The two service methods listRootFolders and listChildren on
MemberFolderService currently return full lists and should support pagination
for scalability; change their signatures to accept a Pageable (e.g.,
List<MemberFolder> listRootFolders(Long ownerMemberId, Pageable pageable) ->
Page<MemberFolder> listRootFolders(Long ownerMemberId, Pageable pageable)) (or
return Slice<MemberFolder>), then update all implementations of
MemberFolderService (and any callers) to use the Pageable param and return a
Page/Slice, and adapt underlying repository calls (e.g.,
repository.findByOwnerIdAndParentIsNull(ownerId, pageable) and
repository.findByOwnerIdAndParentId(ownerId, parentId, pageable)) to pass the
PageRequest so only one page (e.g., size 64 default) is fetched.
- Line 5: The file MemberFolderService.java imports java.util.Optional but does
not use it in the interface method signatures; remove the unused import (delete
the line "import java.util.Optional;") and run a quick compile or IDE organize
imports to ensure no other references to Optional remain in MemberFolderService
or its methods.
In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.java`:
- Around line 17-30: The create method and other mutating methods (update, move,
delete) lack ownerMemberId-based authorization checks, allowing IDOR; add an
ownership validation step in MemberFolderServiceImpl before performing any
update/move/delete (and optionally before read if needed): fetch the target
MemberFolder via memberFolderJpaRepository.findById(memberFolderId) and compare
its ownerMemberId to the caller-provided ownerMemberId, throwing an appropriate
exception (e.g., AccessDeniedException or custom) if they differ; factor this
into a reusable private method like validateOwnerOrThrow(Long ownerMemberId,
Long memberFolderId) and call it from update, move, delete (and any other
mutating methods) to prevent unauthorized operations.
- Around line 63-69: The move(Long memberFolderId, Long newParentFolderId)
method in MemberFolderServiceImpl must validate the new parent: first load the
target parent via memberFolderJpaRepository.findById(newParentFolderId) and
throw FolderException.NotFound if missing; then prevent cycles by rejecting if
newParentFolderId equals memberFolderId or if the target parent is a descendant
of the folder being moved (traverse the parent chain of the loaded parent or
query the folder's subtree to see if any id equals memberFolderId); if
validation fails throw an appropriate FolderException (e.g., InvalidParent)
instead of calling folder.changeParent, otherwise proceed to changeParent.
Ensure checks occur inside the same transactional method so state is consistent.
- Around line 71-78: The delete(Long memberFolderId) currently removes only the
parent and leaves orphaned children; update delete() to handle children using
the existing listChildren(Long parentId) helper: either recursively delete each
child by calling delete(childId) before calling
memberFolderJpaRepository.deleteById(memberFolderId), or iterate children and
set their parentFolderId to null (or the root indicator) and save them via
memberFolderJpaRepository.save/update before deleting the parent; ensure
operations remain within the `@Transactional` boundary and still use
memberFolderJpaRepository.deleteById(...) for removing the parent.
| implementation 'org.springframework.boot:spring-boot-starter-validation' | ||
| implementation 'org.springframework.boot:spring-boot-starter-oauth2-client' | ||
| implementation 'me.paulschwarz:spring-dotenv:4.0.0' | ||
| implementation 'org.springframework.boot:spring-boot-starter-data-jpa' |
There was a problem hiding this comment.
spring-boot-starter-data-jpa 의존성이 중복 선언되어 있습니다.
31번 라인에 이미 동일한 의존성이 선언되어 있으므로, 37번 라인의 중복 항목을 제거해 주세요. 또한 31번 라인의 들여쓰기가 탭이 아닌 스페이스로 되어 있어 파일 내 다른 라인들과 일관성이 맞지 않습니다.
🔧 수정 제안
implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
implementation 'org.mybatis.spring.boot:mybatis-spring-boot-starter:3.0.3'
implementation 'org.thymeleaf.extras:thymeleaf-extras-springsecurity6'
implementation 'org.springframework.boot:spring-boot-starter-validation'
implementation 'org.springframework.boot:spring-boot-starter-oauth2-client'
implementation 'me.paulschwarz:spring-dotenv:4.0.0'
- implementation 'org.springframework.boot:spring-boot-starter-data-jpa'31번 라인의 들여쓰기도 스페이스에서 탭으로 수정하세요:
- implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
+ implementation 'org.springframework.boot:spring-boot-starter-data-jpa'🤖 Prompt for AI Agents
In `@build.gradle` at line 37, Remove the duplicate dependency declaration of
'org.springframework.boot:spring-boot-starter-data-jpa' (keep only one
occurrence) and fix the inconsistent indentation on the remaining declaration by
converting the leading spaces to a tab so it matches the file's existing
indentation style; locate the duplicate by searching for the exact dependency
string and update the earlier occurrence's indentation accordingly.
| public enum CommonErrorCode implements ErrorCode{ | ||
| // 500 Internal Server Error: 서버 내부 오류 | ||
| INTERNAL_SERVER_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, "C006", "서버에 오류가 발생했습니다."); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
에러 코드 "C006" 번호 체계 확인
현재 유일한 상수인 INTERNAL_SERVER_ERROR의 코드가 "C006"입니다. C001~C005에 해당하는 다른 공통 에러 코드가 계획되어 있는지, 아니면 번호 체계에 의도가 있는지 확인이 필요합니다. 향후 유지보수를 위해 코드 번호 규칙을 문서화하는 것을 권장합니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/config/CommonErrorCode.java` around lines 9 -
11, The enum CommonErrorCode currently defines INTERNAL_SERVER_ERROR with code
"C006"; verify the intended numbering scheme and either adjust the code value on
INTERNAL_SERVER_ERROR to the correct sequence (e.g., "C001" if it should be
first) or add/document the missing placeholders for C001–C005 and a comment
explaining the numbering convention; update the enum constant
(INTERNAL_SERVER_ERROR) and associated documentation/comments to reflect the
chosen, consistent code scheme.
| // 그 외 예상치 못한 서버 에러(500) 처리 | ||
| @ExceptionHandler(Exception.class) | ||
| protected ResponseEntity<ApiResponse<Void>> handleException(Exception e) { | ||
| log.error("Internal Server Error", e); | ||
| return ResponseEntity | ||
| .status(HttpStatus.INTERNAL_SERVER_ERROR) | ||
| .body(ApiResponse.fail(CommonErrorCode.INTERNAL_SERVER_ERROR)); | ||
| } |
There was a problem hiding this comment.
IllegalArgumentException이 500으로 처리되는 버그가 있습니다.
MemberFolderServiceImpl.validateFolderName()에서 IllegalArgumentException을 던지는데, 이 예외는 BusinessException이 아니므로 handleException(Exception e)에 잡혀 500 Internal Server Error로 응답됩니다. 클라이언트 입력 검증 실패는 400 Bad Request로 응답해야 합니다.
🐛 IllegalArgumentException 핸들러 추가 제안
+ `@ExceptionHandler`(IllegalArgumentException.class)
+ protected ResponseEntity<ApiResponse<Void>> handleIllegalArgument(IllegalArgumentException e) {
+ log.warn("IllegalArgumentException: {}", e.getMessage());
+ return ResponseEntity
+ .status(HttpStatus.BAD_REQUEST)
+ .body(ApiResponse.fail(CommonErrorCode.INVALID_INPUT));
+ }
+
// 그 외 예상치 못한 서버 에러(500) 처리
`@ExceptionHandler`(Exception.class)또는 validateFolderName에서 BusinessException 계열 예외를 던지도록 서비스 레이어를 수정하는 것도 방법입니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/config/GlobalExceptionHandler.java` around
lines 26 - 33, Add a specific handler for IllegalArgumentException in
GlobalExceptionHandler so client validation errors from
MemberFolderServiceImpl.validateFolderName() return 400 instead of being caught
by the generic handleException(Exception). Implement a new method (e.g.,
handleIllegalArgumentException) annotated with
`@ExceptionHandler`(IllegalArgumentException.class) that logs at WARN/INFO and
returns
ResponseEntity.status(HttpStatus.BAD_REQUEST).body(ApiResponse.fail(...)) using
an appropriate client-error CommonErrorCode (or add one if missing);
alternatively, change MemberFolderServiceImpl.validateFolderName() to throw a
BusinessException subclass so the existing BusinessException handler handles it.
| public class MemberFolderRequests { | ||
|
|
||
| /** | ||
| * 폴더 생성 요청 | ||
| */ | ||
| public static class Create { | ||
| public Long ownerMemberId; | ||
| public Long parentFolderId; // null이면 루트 | ||
| public String folderName; | ||
| public String description; | ||
| } | ||
|
|
||
| /** | ||
| * 폴더 수정 요청 | ||
| */ | ||
| public static class Update { | ||
| public String folderName; | ||
| public String description; | ||
| } | ||
|
|
||
| /** | ||
| * 폴더 이동 요청 | ||
| */ | ||
| public static class Move { | ||
| public Long newParentFolderId; // null이면 루트로 이동 | ||
| } |
There was a problem hiding this comment.
요청 DTO에 Bean Validation 어노테이션 누락
모든 inner class 필드가 public으로 선언되어 있고, @NotBlank, @NotNull, @Size 등의 Bean Validation 어노테이션이 없습니다. 도메인 엔티티의 changeInfo에서 folderName null/blank 검증을 하고 있지만, 컨트롤러 진입 시점에서 조기 실패(fail-fast)하는 것이 올바른 패턴입니다.
♻️ Validation 적용 및 캡슐화 개선 제안
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.Size;
+import lombok.Getter;
+import lombok.Setter;
+
public class MemberFolderRequests {
public static class Create {
- public Long parentFolderId;
- public String folderName;
- public String description;
+ `@Getter` `@Setter`
+ private Long parentFolderId;
+
+ `@NotBlank`
+ `@Size`(max = 100)
+ `@Getter` `@Setter`
+ private String folderName;
+
+ `@Size`(max = 500)
+ `@Getter` `@Setter`
+ private String description;
}컨트롤러의 @RequestBody 앞에 @Valid를 추가해야 합니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/web/SearchWeb/folder/controller/dto/MemberFolderRequests.java`
around lines 3 - 28, The DTOs in MemberFolderRequests (inner classes Create,
Update, Move) lack Bean Validation and use public mutable fields; change each
DTO to encapsulate fields (make them private and provide getters/setters or
apply Lombok annotations like `@Getter/`@Setter/@NoArgsConstructor) and add
appropriate javax.validation annotations: for Create.folderName use `@NotBlank`
and `@Size`(max=...) (and `@NotNull` for ownerMemberId), for Update.folderName use
`@NotBlank/`@Size as needed, and for Move.newParentFolderId use `@NotNull` if moving
is required (or omit if null allowed); finally ensure controller methods that
accept these DTOs annotate the `@RequestBody` parameters with `@Valid` so validation
runs on entry.
| public static class Create { | ||
| public Long ownerMemberId; | ||
| public Long parentFolderId; // null이면 루트 | ||
| public String folderName; | ||
| public String description; | ||
| } |
There was a problem hiding this comment.
보안 문제: ownerMemberId를 요청 본문에서 받으면 안 됩니다
Create DTO에서 ownerMemberId를 클라이언트 요청 본문으로 받고 있습니다. 악의적인 사용자가 다른 사용자의 ID를 지정하여 해당 사용자 계정에 폴더를 생성할 수 있는 IDOR(Insecure Direct Object Reference) 취약점입니다.
ownerMemberId는 인증된 세션/토큰(예: SecurityContextHolder, @AuthenticationPrincipal)에서 추출해야 합니다.
🔒 Create DTO에서 ownerMemberId 제거 제안
public static class Create {
- public Long ownerMemberId;
public Long parentFolderId; // null이면 루트
public String folderName;
public String description;
}컨트롤러에서 인증 정보로부터 ownerMemberId를 주입하는 방식으로 변경하세요.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static class Create { | |
| public Long ownerMemberId; | |
| public Long parentFolderId; // null이면 루트 | |
| public String folderName; | |
| public String description; | |
| } | |
| public static class Create { | |
| public Long parentFolderId; // null이면 루트 | |
| public String folderName; | |
| public String description; | |
| } |
🤖 Prompt for AI Agents
In
`@src/main/java/com/web/SearchWeb/folder/controller/dto/MemberFolderRequests.java`
around lines 8 - 13, Remove the ownerMemberId field from the
MemberFolderRequests.Create DTO (so it is no longer accepted from the request
body) and update the folder creation flow to derive the owner ID from the
authenticated principal instead (e.g., obtain memberId from
SecurityContextHolder or an `@AuthenticationPrincipal` argument in the controller
method that handles Create). Ensure the controller no longer reads
Create.ownerMemberId but passes the principal-derived memberId into the
service/repository create method (keeping parentFolderId, folderName,
description on Create unchanged) so owner assignment is done server-side and
IDOR is eliminated.
|
|
||
| import com.web.SearchWeb.folder.domain.MemberFolder; | ||
| import java.util.List; | ||
| import java.util.Optional; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
사용되지 않는 Optional import
java.util.Optional이 import되어 있으나 인터페이스 메서드 시그니처에서 사용되지 않습니다.
♻️ 미사용 import 제거
-import java.util.Optional;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import java.util.Optional; |
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderService.java` at
line 5, The file MemberFolderService.java imports java.util.Optional but does
not use it in the interface method signatures; remove the unused import (delete
the line "import java.util.Optional;") and run a quick compile or IDE organize
imports to ensure no other references to Optional remain in MemberFolderService
or its methods.
| List<MemberFolder> listRootFolders(Long ownerMemberId); | ||
|
|
||
| List<MemberFolder> listChildren(Long ownerMemberId, Long parentFolderId); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
목록 조회 메서드에 페이징 미지원
listRootFolders와 listChildren이 전체 목록을 반환합니다. PR 설명에 최대 64개 폴더 제한이 언급되어 있으므로 현재는 문제가 없을 수 있지만, 향후 확장성을 고려하면 Pageable 파라미터 지원을 검토할 수 있습니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderService.java`
around lines 13 - 15, The two service methods listRootFolders and listChildren
on MemberFolderService currently return full lists and should support pagination
for scalability; change their signatures to accept a Pageable (e.g.,
List<MemberFolder> listRootFolders(Long ownerMemberId, Pageable pageable) ->
Page<MemberFolder> listRootFolders(Long ownerMemberId, Pageable pageable)) (or
return Slice<MemberFolder>), then update all implementations of
MemberFolderService (and any callers) to use the Pageable param and return a
Page/Slice, and adapt underlying repository calls (e.g.,
repository.findByOwnerIdAndParentIsNull(ownerId, pageable) and
repository.findByOwnerIdAndParentId(ownerId, parentId, pageable)) to pass the
PageRequest so only one page (e.g., size 64 default) is fetched.
| @Override | ||
| @Transactional | ||
| public Long create(Long ownerMemberId, Long parentFolderId, String folderName, String description) { | ||
| validateFolderName(folderName); | ||
|
|
||
| MemberFolder folder = MemberFolder.builder() | ||
| .ownerMemberId(ownerMemberId) | ||
| .parentFolderId(parentFolderId) | ||
| .folderName(folderName) | ||
| .description(description) | ||
| .build(); | ||
|
|
||
| return memberFolderJpaRepository.save(folder).getMemberFolderId(); | ||
| } |
There was a problem hiding this comment.
모든 변경 작업에서 ownerMemberId 기반 권한 검증이 없습니다.
create를 제외한 update, move, delete 메서드에서 요청자가 해당 폴더의 소유자인지 확인하지 않습니다. 현재 구조에서는 folderId만 알면 다른 사용자의 폴더를 수정/삭제할 수 있는 IDOR(Insecure Direct Object Reference) 취약점이 존재합니다.
Also applies to: 51-61, 63-69, 71-78
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.java`
around lines 17 - 30, The create method and other mutating methods (update,
move, delete) lack ownerMemberId-based authorization checks, allowing IDOR; add
an ownership validation step in MemberFolderServiceImpl before performing any
update/move/delete (and optionally before read if needed): fetch the target
MemberFolder via memberFolderJpaRepository.findById(memberFolderId) and compare
its ownerMemberId to the caller-provided ownerMemberId, throwing an appropriate
exception (e.g., AccessDeniedException or custom) if they differ; factor this
into a reusable private method like validateOwnerOrThrow(Long ownerMemberId,
Long memberFolderId) and call it from update, move, delete (and any other
mutating methods) to prevent unauthorized operations.
| @Override | ||
| @Transactional | ||
| public void move(Long memberFolderId, Long newParentFolderId) { | ||
| MemberFolder folder = memberFolderJpaRepository.findById(memberFolderId) | ||
| .orElseThrow(FolderException.NotFound::new); | ||
| folder.changeParent(newParentFolderId); | ||
| } |
There was a problem hiding this comment.
move()에 순환 참조 방지 로직이 없습니다.
폴더를 자기 자신이나 자신의 하위 폴더로 이동시키면 트리 구조에 순환(cycle)이 생겨 조회 시 무한 루프나 데이터 정합성 문제가 발생합니다. newParentFolderId가 해당 폴더의 하위 트리에 속하지 않는지 검증이 필요합니다.
또한 newParentFolderId가 실제로 존재하는 폴더인지 검증하지 않아, 존재하지 않는 부모에 연결될 수 있습니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.java`
around lines 63 - 69, The move(Long memberFolderId, Long newParentFolderId)
method in MemberFolderServiceImpl must validate the new parent: first load the
target parent via memberFolderJpaRepository.findById(newParentFolderId) and
throw FolderException.NotFound if missing; then prevent cycles by rejecting if
newParentFolderId equals memberFolderId or if the target parent is a descendant
of the folder being moved (traverse the parent chain of the loaded parent or
query the folder's subtree to see if any id equals memberFolderId); if
validation fails throw an appropriate FolderException (e.g., InvalidParent)
instead of calling folder.changeParent, otherwise proceed to changeParent.
Ensure checks occur inside the same transactional method so state is consistent.
| @Override | ||
| @Transactional | ||
| public void delete(Long memberFolderId) { | ||
| if (!memberFolderJpaRepository.existsById(memberFolderId)) { | ||
| return; // 멱등 삭제 | ||
| } | ||
| memberFolderJpaRepository.deleteById(memberFolderId); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "MemberFolder.java" --type f -x cat {}Repository: Searchweb-Dev/Searchweb-Back
Length of output: 1517
🏁 Script executed:
fd "MemberFolderJpaRepository" --type f -x cat {}Repository: Searchweb-Dev/Searchweb-Back
Length of output: 54
🏁 Script executed:
rg "parentFolderId" --type java -A 3 -B 3Repository: Searchweb-Dev/Searchweb-Back
Length of output: 10375
🏁 Script executed:
fd "MemberFolderServiceImpl.java" --type f -x cat -n {}Repository: Searchweb-Dev/Searchweb-Back
Length of output: 3591
부모 폴더 삭제 시 자식 폴더가 고아(orphan) 레코드가 되는 데이터 무결성 문제가 있습니다.
delete() 메서드에서 부모 폴더만 삭제하고 자식 폴더를 처리하지 않으므로, 자식 폴더들의 parentFolderId가 존재하지 않는 ID를 가리키게 됩니다. 코드에 이미 listChildren() 메서드가 있으므로, 삭제 시 다음 중 하나의 전략을 구현해야 합니다:
- 자식 폴더를 재귀적으로 삭제
- 자식 폴더의 부모를 루트(null)로 설정하여 승격
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.java`
around lines 71 - 78, The delete(Long memberFolderId) currently removes only the
parent and leaves orphaned children; update delete() to handle children using
the existing listChildren(Long parentId) helper: either recursively delete each
child by calling delete(childId) before calling
memberFolderJpaRepository.deleteById(memberFolderId), or iterate children and
set their parentFolderId to null (or the root indicator) and save them via
memberFolderJpaRepository.save/update before deleting the parent; ensure
operations remain within the `@Transactional` boundary and still use
memberFolderJpaRepository.deleteById(...) for removing the parent.
💡 이슈
resolve SW-40
🤩 개요
PR의 개요를 적어주세요.
🧑💻 작업 사항
작업한 내용을 적어주세요.
📖 참고 사항
공유할 내용, 레퍼런스, 추가로 발생할 것으로 예상되는 이슈, 스크린샷 등을 넣어 주세요.
세부 로직에 대한 추가 들이 필요함.
ex) 폴더를 64개 까지 만들수 있다든가 하는...
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
개선 사항